See #39 and #23. Any reason we should revisit the decision to allow_failures now?

See #39 and #23. Any reason we should revisit the decision to allow_failures now?

The main reason is that we have it in the CI, but allowing it to fail means we don’t notice it (like in #104) , so it gets missed. For me, the issue is that I autoformat the code as I write it, so I end up with unrelated changes when I edit a misformatted file.

On my end, the issue isn’t that bad, but it’s also not a huge deal to require PRs to just run cargo fmt. I know @newpavlov brought up a (valid) point about stability, but that’s virtually always an issue with the nightly builds of rustfmt (in that they sometimes fail, breaking the CI), but we use stable so that particular issue won’t affect us.

Thoughts?

You could get used to typing cargo fmt and committing the result before making any other changes. A pain I know, but any way about it, half the devs using rustfmt and the other not is a pain for someone.

But since you've been doing far more work on this crate than me I'll give you priority on this, if @newpavlov agrees.

In my experience it's PITA to deal with rustfmt failures in CI, especially for outside contributors. And not everyone (e.g. me) uses stable toolchain for development. While I can live with the proposed change, I think fixing formatting from time to time (e.g. as part of release process) is a slightly better approach.

Then the decision seems clear: we continue to allow failures for rustfmt.

Rebase without that part @josephlr.

Rebase without that part @josephlr.

Done, updated title and description.

Thanks for the clarification @newpavlov